Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use the refactored version of soroban-simulation library #92

Merged
merged 2 commits into from
Mar 6, 2024

Conversation

dmkozh
Copy link
Contributor

@dmkozh dmkozh commented Mar 1, 2024

The refactored library encapsulates the network configuration in the library, so there is no longer a need to track the bucket list size from soroban-rpc. It also accepts the adjustment config that allows customizing the resource/refundable fee leeway without modifying the simulation crate itself.

Besides the refactoring, soroban-simulation library is switched to use a more accurate recording mode implementation, which allows to decrease the necessary CPU instruction leeway significantly.

@dmkozh dmkozh force-pushed the sim_refactor branch 2 times, most recently from c7642db to 4249ca7 Compare March 5, 2024 21:34
@dmkozh dmkozh marked this pull request as ready for review March 5, 2024 22:36
The refactored library encapsulates the network configuration in the library, so there is no longer a need to track the bucket list size from soroban-rpc. It also accepts the adjustment config that allows customizing the resource/refundable fee leeway without modifying the simulation crate itself.

Besides the refactoring, soroban-simulation library is switched to use a more accurate recording mode implementation, which allows to decrease the necessary CPU instruction leeway significantly.
@dmkozh dmkozh changed the title Simulation refactor Use the refactored version of soroban-simulation library Mar 5, 2024
Copy link
Contributor

@2opremio 2opremio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

The internal_error hack is pretty ugly (it would be way cleaner if an appropriate or custom HostErrors could be used instead).

@dmkozh
Copy link
Contributor Author

dmkozh commented Mar 6, 2024

The internal_error hack is pretty ugly (it would be way cleaner if an appropriate or custom HostErrors could be used instead).

FWIW internal error is probably the most appropriate thing we can return currently as simulation will be incorrect in case if storage is somehow broken. It would be tricky to introduce new host error codes that are exclusive to simulation only (probably it's not impossible, but pretty cumbersome).

@dmkozh dmkozh merged commit ad98bb7 into stellar:main Mar 6, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants